feat(cli): vouch new <kind> — scaffold a typed page/entity proposal (closes #330)#346
feat(cli): vouch new <kind> — scaffold a typed page/entity proposal (closes #330)#346e11734937-beep wants to merge 2 commits into
Conversation
Closes vouchdev#330. Add `vouch new <kind>`: read the page-kind registry (or EntityType) for KIND, stub every required field, and file a pending proposal through the normal review gate — composing the existing propose_page / propose_entity calls, with no new kb.* method, storage logic, or server registration.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Changesvouch new scaffold command
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Author
participant CLI as vouch new
participant Registry as PageKindRegistry
participant Proposals as propose_page/propose_entity
participant Store as KBStore
Author->>CLI: vouch new <kind> [flags]
CLI->>Registry: resolve(kind)
alt entity path
CLI->>Proposals: propose_entity(store, ...)
else page path
CLI->>CLI: parse fields, fill missing values
CLI->>Proposals: propose_page(store, ..., dry_run?)
end
Proposals->>Store: create pending proposal
Store-->>CLI: proposal id
CLI-->>Author: print id or draft
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_new_scaffold.py (2)
41-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
--interactiveprompting andrequired_citationsreminder.The PR objectives call out two behaviors that have no corresponding test here:
--interactiveprompting for missing required fields, and a citation reminder surfaced for kinds withrequired_citations. Consider adding tests for both to lock in the documented behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_new_scaffold.py` around lines 41 - 126, Add test coverage in test_new_scaffold for the two missing scaffold behaviors: the CliRunner-based `new` command should prompt for absent required fields when `--interactive` is used, and kinds declared with `required_citations` should surface a reminder in the output. Reuse the existing scaffolding helpers like `_declare_kind`, `cli`, and `store.get_proposal` to assert the interactive path fills or requests missing required data, and verify the citation reminder appears for the affected kind.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFile name doesn't mirror the module under test.
This suite tests the
newcommand added tosrc/vouch/cli.py, but the file is namedtest_new_scaffold.pyrather thantest_cli.py. As per coding guidelines, "Test file names must mirror module names using thetests/test_<module>.pyconvention."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_new_scaffold.py` at line 1, The test file name does not match the module under test, so rename the suite to follow the tests/test_<module>.py convention for src/vouch/cli.py. Update the test filename from test_new_scaffold.py to the corresponding cli-focused name so it clearly mirrors the module being exercised, and keep the existing test contents unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/cli.py`:
- Around line 1098-1101: The page-kind registry lookup in the CLI setup is
happening outside the existing _cli_errors() handling, so invalid page-kind
configuration can still surface a traceback and stop entity-only scaffolds. Move
the load_page_kind_registry(store) and subsequent
registry.known()/registry.resolve(kind) usage into the _cli_errors() block in
the vouch new flow, and keep the exception handling focused there rather than
adding PageKindError separately since it already inherits ValueError.
---
Nitpick comments:
In `@tests/test_new_scaffold.py`:
- Around line 41-126: Add test coverage in test_new_scaffold for the two missing
scaffold behaviors: the CliRunner-based `new` command should prompt for absent
required fields when `--interactive` is used, and kinds declared with
`required_citations` should surface a reminder in the output. Reuse the existing
scaffolding helpers like `_declare_kind`, `cli`, and `store.get_proposal` to
assert the interactive path fills or requests missing required data, and verify
the citation reminder appears for the affected kind.
- Line 1: The test file name does not match the module under test, so rename the
suite to follow the tests/test_<module>.py convention for src/vouch/cli.py.
Update the test filename from test_new_scaffold.py to the corresponding
cli-focused name so it clearly mirrors the module being exercised, and keep the
existing test contents unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b95783b5-a956-4f87-bd3f-8746d09ce694
📒 Files selected for processing (2)
src/vouch/cli.pytests/test_new_scaffold.py
load_page_kind_registry() and registry.resolve() ran outside the _cli_errors() handler in `vouch new`, so an invalid page_kinds config surfaced a raw traceback and could block entity-only scaffolds. Move the registry load/known()/resolve() calls inside _cli_errors() so PageKindError (already a ValueError subclass) is translated to a clean CLI error. Success path is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hi @plind-junior, Thanks for building vouch — nice to see a tool aimed squarely at the SN74 contribution loop. I have two PRs open that are waiting on first-time-contributor approval before CI can run:
Both are ready for review. I verified them locally against the CI gates — Whenever you get a moment, would you mind approving the workflow run and taking a look? Happy to adjust anything to fit the repo conventions. Thanks! |
|
Hi @plind-junior — following up on #346 and #347: they are still waiting on first-contributor CI approval before the checks can run. Both are ready — I verified them locally against the CI gates ( |
Summary
Closes #330. Adds
vouch new <kind>— scaffolds a typed page or entity proposal from its registered shape, so an author gets a correctly-shaped draft instead of hand-writing--metapairs and discovering required fields only whenpropose-pagerejects them.It reads
load_page_kind_registry(store)for the kind, stubs every required field, and files a pending proposal via the existingpropose_page/propose_entitycalls — no newkb.*method, nostorage.pylogic, no server/JSONL registration. Just the CLI command insrc/vouch/cli.pyplustests/test_new_scaffold.py.Surface
vouch new <kind> --title <t>for page kinds;--name <n>for entity kinds.--field key=value(repeatable, YAML-parsed through the same_parse_metapath as--metaonpropose-page).--interactive/-i— prompt for each unfilled required field (default off, so runs stay scriptable).--body <t>/--body -(stdin),--dry-run,--json,--claim/--source,--alias/--description.--entityforces the entity path; a name that is only anEntityTypescaffolds an entity; thedecision/projectcollisions resolve to page unless--entity; an unknown kind fails listing the known page kinds and entity types.Review gate & validation (design note)
The scaffold never weakens validation. It calls
propose_page(..., proposed_by=_whoami())exactly aspropose-pagedoes, so the samepage_kindsand dangling-reference checks run. Becausepropose_pagere-validates (_is_missingtreats an empty value as missing), a required field left empty is flagged the same way any other proposal is — you fill it via--field/--interactive, or see it listed in--dry-run.--dry-runassembles and prints the draft (title, kind, stubbed frontmatter, missing-required list) and files nothing. Arequired_citationskind surfaces a citation reminder rather than silently proposing an uncited page. The scaffolded artifact is pending-only — a human still runsvouch approve <id>to land it.(If the intent of "proposable but flagged" was to file a pending proposal even with empty required fields rather than surface the validation error, that's a one-line change — happy to adjust; I defaulted to faithfully reusing
propose_page's existing validation.)Tests
tests/test_new_scaffold.pycovers page scaffold (create with required filled), dry-run stubbing (no write), unfilled-required flagging,--fieldYAML parsing, entity scaffold routing, collision dispatch (decision→ page;--entity→ entity), unknown-kind error, and pending-only.Verification
python -m ruff check src tests— cleanpython -m mypy src— cleanpython -m pytest— full suite green (8 new tests; no regressions)Summary by CodeRabbit
New Features
vouch newcommand to scaffold pending proposals for page or entity kinds.--dry-run,--json, interactive filling of missing required fields, and--field key=valueYAML parsing.--body -.Bug Fixes